Initialize CPU usage #1725
Initialize CPU usage #1725
Conversation
Codecov Report
@@ Coverage Diff @@
## develop #1725 +/- ##
===========================================
+ Coverage 68.16% 68.17% +0.01%
===========================================
Files 80 80
Lines 11609 11617 +8
Branches 1631 1635 +4
===========================================
+ Hits 7913 7920 +7
Misses 3359 3359
- Partials 337 338 +1
Continue to review full report at Codecov.
|
| def get_cpu_usage(self): | ||
| """ | ||
| Collects and return the cpu usage. | ||
| cgroup_delta = self._current_cgroup_cpu - self._previous_cgroup_cpu |
There was a problem hiding this comment.
We don't need the absolute value here?
There was a problem hiding this comment.
Update to my comment: I assume that since we care about the ratio of the deltas, and assuming that if the cgroup delta is negative, then the system one will be, they will cancel each other out.
I'm not sure this is always the case though, that the sign is always the same for both deltas?
There was a problem hiding this comment.
The CPU usage is measured in ticks. More recent values would never be less than older values so the difference (both for cgroup and system) would always be positive (or zero).
| def _cpu_usage_initialized(self): | ||
| return self._current_cgroup_cpu is not None and self._current_system_cpu is not None | ||
|
|
||
| def initialize_cpu_usage(self): |
There was a problem hiding this comment.
Can we call initialize_cpu_usage within the __init__ itself? Asking for my knowledge.
There was a problem hiding this comment.
The get*cgroup functions instantiate cgroups that may never be tracked... I thought it was not worth initializing the cpu usage for those (especially since the actual cgroups may not exist on disk yet --- if we initialize on init then we would need to ensure the cgroups exist before calling the getter).
| @@ -1354,7 +1354,7 @@ def get_total_cpu_ticks_since_boot(): | |||
| if proc_stat is not None: | |||
| for line in proc_stat.splitlines(): | |||
| if ALL_CPUS_REGEX.match(line): | |||
| system_cpu = sum(int(i) for i in line.split()[1:7]) | |||
| system_cpu = sum(int(i) for i in line.split()[1:8]) | |||
There was a problem hiding this comment.
Can you add a comment on what the columns are?
There was a problem hiding this comment.
OK - I'll add a comment referencing what man page can be consulted (description can be sort of long to include here)
The CPU usage for the cgroup was not initialized when creating the CpuCgroup (the system usage was).
Now initialization sets both values and is done when we start tracking the cgroup (as opposed as when it is created). We can create cgroups and never track them so this is a better place for initialization.
Now CPU is computed as a percentage of usage for all cores (e.g. if a cgroup was using 2 cores at 100% on a 4-core system, now we report 50% instead of 200%). This makes data easier to use.
Also, added field #8 of /proc/stat to the computation of CPU (see "man proc"):
This change is